Skip to content

fix(geocoding): tap targets for Navigate (peek state) + close dropdown on Navigate here (#177)#178

Merged
jasoneplumb merged 2 commits intomainlinefrom
fix/nav-pointer-events
Apr 26, 2026
Merged

fix(geocoding): tap targets for Navigate (peek state) + close dropdown on Navigate here (#177)#178
jasoneplumb merged 2 commits intomainlinefrom
fix/nav-pointer-events

Conversation

@jasoneplumb
Copy link
Copy Markdown
Owner

@jasoneplumb jasoneplumb commented Apr 26, 2026

Summary

Two silently-swallowed taps that combine to make both Navigate and Stop appear broken on mobile.

1. Geocode-bar Navigate missing from peek opt-in (long-press flow)

```css
.geocode-bar--peek { pointer-events: none; }
.geocode-bar--peek .geocode-bar__handle,
.geocode-bar--peek .geocode-bar__copy,
.geocode-bar--peek .geocode-bar__close,
.geocode-bar--peek .geocode-bar__addr { pointer-events: auto; }
```

`.geocode-bar__nav` is missing from this list. The CSS spec says descendants of `pointer-events: none` still receive events, but mobile Safari has long-standing quirks here — the existing opt-in for the other interactive children was already a workaround for this. Add Navigate to it.

This also explains why the Stop fix from #176 didn't help: `hideGeocodeBar()` is called from Navigate's click handler. If Navigate never fires, the bar never hides, and the bar's drag handle keeps eating Stop taps.

2. Search dropdown stays open after Navigate here

The `.sheet-result__nav-btn` handler at `geocoding.ts:246` calls `startGuidance()` without closing the dropdown. `.search-dropdown` is `z-index: 2000` (above the guidance pill at Leaflet's default ~800), and on small viewports can overlap the pill. Add `hideDropdown()` before guidance starts.

Closes #177.

Test plan

  • `npm run type-check` clean
  • `npm run lint` clean
  • `npm test` clean (633)
  • Manual: long-press → Navigate (bar) → button responds, bar hides, guidance starts
  • Manual: long-press → Navigate → Stop responds
  • Manual: search → Navigate here (dropdown) → dropdown closes, guidance starts
  • Manual: search → Navigate here → Stop responds

🤖 Generated with Claude Code

@claude
fix(geocoding): make Navigate tappable in peek + close search dropdow…
07cd8e5 
…n on Navigate here

Two related taps were silently swallowed.

1. Geocode-bar Navigate button missing from peek pointer-events
   opt-in. The bar in peek state has pointer-events: none and only
   handle/copy/close/addr were explicitly opted back in. On mobile
   Safari the implicit child-receives-events behaviour for a
   pointer-events: none parent is unreliable, so .geocode-bar__nav
   silently dropped taps. Add it to the opt-in list.

   Without Navigate firing, hideGeocodeBar() from #176 never ran,
   so the bar stayed visible and the guidance pill's Stop button
   was still under the bar's drag handle.

2. Search dropdown's Navigate here handler started guidance without
   closing the dropdown. The dropdown sits at z-index 2000 above
   the guidance pill and on smaller viewports can overlap its
   action area; also a UX nit (stale chrome). Add hideDropdown()
   to the click handler.

Closes #177

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jasoneplumb jasoneplumb added bug Something isn't working P1 Critical — fix immediately review-requested Triggers Claude code review labels Apr 26, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 26, 2026

Code Review

Overall: Clean, minimal fix that correctly diagnoses two related bugs. The root-cause analysis in the PR description is excellent — the cascade (Navigate tap swallowed → hideGeocodeBar() never called → drag handle eats Stop taps) explains both #176 and #177 in one shot.

src/style.css — .geocode-bar__nav pointer-events opt-in

  • Correct. The missing entry from the pointer-events: auto allowlist is the precise fix needed for mobile Safari's quirky inheritance of pointer-events: none.
  • The insertion position (between __handle and __copy) preserves visual/DOM order rather than alphabetical order, which is fine — CSS selector order has no effect on specificity here.
  • No concerns.

src/geocoding.ts — hideDropdown() before startGuidance()

  • Correct. Hiding the dropdown synchronously before launching the async navigation is the right sequencing — the dropdown is gone before any guidance pill appears.
  • void startGuidance(...) was already present, so the async intent is preserved.

Minor style nit (per project CLAUDE.md): the inline comment references (#177) — the project convention is to keep issue references in commit messages/PR descriptions rather than inline comments, since they rot as the codebase evolves. The why ("can't overlap the guidance pill") is worth keeping; the issue number reference isn't. Consider:

// Hide dropdown before guidance pill appears to avoid z-index overlap.
hideDropdown();

This is genuinely minor and not a blocker.

Test coverage

No unit tests added, which is appropriate — these are pointer-event/z-index interaction bugs that can't be exercised by vitest pure-function tests. The 4 manual test cases in the test plan are the right verification path.

Summary

  • CSS fix: approve as-is
  • TS fix: approve as-is (inline (#177) is a trivial style nit, not a blocker)
  • No correctness, performance, or security concerns

The fix is well-scoped and the PR description is thorough. Ready to merge once manual test cases are verified.

@claude
fix: drop issue ref from inline comment per CLAUDE.md
00104a1 
Per PR #178 review nit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jasoneplumb jasoneplumb merged commit a1b2c0b into mainline Apr 26, 2026
2 checks passed
@jasoneplumb jasoneplumb deleted the fix/nav-pointer-events branch April 26, 2026 06:17
@claude
Copy link
Copy Markdown

claude Bot commented Apr 26, 2026

Code Review

Overall: Clean, surgical fix for two related mobile tap-event bugs. The change is small, well-scoped, and the PR description does excellent work explaining the causal chain between them.

What the PR does

  1. CSS — adds .geocode-bar__nav to the pointer-events: auto opt-in list inside .geocode-bar--peek, matching the pattern already in place for __handle, __copy, __close, and __addr.
  2. JS — calls hideDropdown() before startGuidance() in the search-dropdown Navigate handler, preventing z-index overlap with the guidance pill on small viewports.

Code Quality

  • Both changes are minimal and exactly scoped to the bug — no unrelated cleanup or refactoring has crept in.
  • The new comment in geocoding.ts explains why the ordering matters (z-index 2000), which is the right kind of comment per project conventions.
  • CSS selector insertion order (handle, nav, copy, close, addr) is consistent with the existing list.

Correctness

  • The CSS fix is clearly right: __nav was simply omitted from a list where all other interactive children in peek state were already present.
  • Calling hideDropdown() before startGuidance() is the correct order — the dropdown needs to be gone before the guidance pill enters the DOM so there's no frame where they overlap.
  • The root-cause chain documented in the PR description (Navigate never fires → bar never hides → drag handle blocks Stop) is coherent and consistent with the code.

Minor observations (non-blocking)

  • void startGuidance(...) silently discards any rejection. This is pre-existing pattern, not introduced here, but if startGuidance fails after hideDropdown() has already run, the dropdown stays closed (which is probably fine — the user would need to re-search anyway). Worth noting for future error-handling work.
  • The z-index value 2000 is referenced in the comment but is a magic number defined elsewhere in the CSS. Not a problem for this patch, just a future cleanup candidate.

Test Coverage

Unit tests (633) pass. The two behaviors fixed here are pointer-event and z-index interactions that can only be verified manually on a touch device — no actionable gap in the automated suite.

Verdict: LGTM. Both fixes are correct, well-explained, and consistent with project conventions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug Something isn't working P1 Critical — fix immediately review-requested Triggers Claude code review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix: Navigate button (geocode-bar) missing from peek pointer-events opt-in; search dropdown stays open after Navigate here

1 participant

@jasoneplumb